-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[fix] function execution events accept interfaces not strings #1357
base: master
Are you sure you want to change the base?
Conversation
Code change seems simple enough. I updated one of the automated tests to test a variety of values for the input, including the |
Oh I actually had cases to cover this but I guess I forgot to push. I'll incorporate b005e63 as well. Thanks! |
@calebmckay are we good to land this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ProjectBarks I can approve, but I'm not actually a maintainer - just kind of ad-hoc helping with some code changes and PRs.
@lorenzoaiello Is this good to merge?
@parsley42 can you take a look? |
It’s good with me, but @nlopes has taken back over maintainership of the repo, so I’ll defer to him until I get an idea of how he wants to run things moving forward. |
Apologies for the lack of action here, I'm currently OOO but should get to this early next week 🙏 |
API changes [BREAKING]
In the initial implementation of this change, we assumed that the input types would always be
map[string]string
. However, Slack can send inputs with various types, not just strings. This update modifies the mapping to use aninterface{}
to handle these different input types. Additionally, the test has been updated to include an example of a real payload received from Slack to ensure proper handling of these cases.Example